-
Notifications
You must be signed in to change notification settings - Fork 4
983 new external page design implementation WIP #1022
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## staging #1022 +/- ##
===========================================
- Coverage 13.09% 13.05% -0.04%
===========================================
Files 450 450
Lines 3108 3117 +9
===========================================
Hits 407 407
- Misses 2701 2710 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
* start with board page * board page is done * lint fix * lint fix * lint fix * lint fix
* identity page done * small responsive fix * lint fix * lint fix * lint fix
WalkthroughAdds four new public routes (board, identity, flux, partners) with templates, translations, styles, and tests; introduces a reusable Public::Card component and replaces static about-alpha content with card usage; updates header navigation to include new links and partners path; adds SCSS imports and styling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant HeaderNav as HeaderNav (component)
participant Router
participant Route as Public Route
participant Template as Public Template
participant Card as Public::Card / Header
User->>HeaderNav: click "Board/Identity/Flux/Partners"
HeaderNav->>Router: transitionTo('public.*')
Router->>Route: instantiate route
Route-->>Template: render template
Template->>Card: render Public::Header and Public::Card(s)
Card-->>User: display composed page (content from translations)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/styles/components/public/index/about-alpha.scss (1)
2-5
: Remove unused.about-text
selector.The template now uses the class
public-text
(line 5 of about-alpha.hbs), so this.about-text
rule appears to be unused and should be removed to avoid dead CSS.Apply this diff to remove the unused rule:
.public-index-about-alpha { - .about-text { - font-size: 1.5rem; - hyphens: none; - } - .about-v{ padding: 0 5%; } }
🧹 Nitpick comments (13)
tests/unit/routes/public/partners-test.js (1)
7-10
: Consider expanding test coverage beyond existence checks.While this existence test is standard and acceptable for initial development, consider adding tests for route behavior, model hooks, or transitions in future iterations to ensure comprehensive coverage.
tests/unit/routes/public/board-test.js (1)
7-10
: Consider expanding test coverage beyond existence checks.As with the partners route test, consider adding tests for route-specific behavior in future iterations for more comprehensive coverage.
app/components/public/card.hbs (1)
1-8
: Consider standardizing prop naming to camelCase.The
@imgurl
prop uses lowercase, which is inconsistent with JavaScript/Ember conventions. Consider renaming to@imageUrl
for better consistency across the codebase.tests/integration/components/public/card-test.js (1)
9-22
: Consider expanding test coverage.While the current tests verify basic rendering, consider adding test cases for:
- The
@horizontal
prop (verify conditional class application)- Image rendering with
@imgurl
- All props rendered together to ensure they don't conflict
Example additions:
test('it applies horizontal class', async function (assert) { await render(hbs`<Public::Card @horizontal={{true}} />`); assert.dom('.circle-card').hasClass('horizontal'); await render(hbs`<Public::Card @horizontal={{false}} />`); assert.dom('.circle-card').hasClass('vertical'); }); test('it renders image', async function (assert) { await render(hbs`<Public::Card @imgurl="/test.jpg" />`); assert.dom('.card-image').hasAttribute('src', '/test.jpg'); });app/styles/components/public/card.scss (1)
55-59
: Consider removing text-align: justify.The
text-align: justify
property can create uneven word spacing and rivers of whitespace, which reduces readability, especially for shorter text blocks. Modern web typography generally favors left-aligned text for better readability.Apply this diff:
.card-text { margin-top: 1rem; - text-align: justify; + text-align: left; font-size: 1.5rem; }app/routes/public/board.js (1)
1-3
: Consider removing empty route class if no logic is needed.Empty route classes can typically be omitted unless you plan to add model hooks, lifecycle methods, or other route-specific logic later. Ember will automatically generate the route if only the template exists.
app/styles/routes/public.scss (2)
29-29
: Add space before opening brace for consistency.The selector is missing a space before the opening brace, which is inconsistent with the rest of the codebase styling.
Apply this diff:
- .red-container{ + .red-container {
15-15
: Consider responsive width constraint for mobile devices.Setting
.public-text
towidth: 80vw
might cause horizontal scrolling or awkward layouts on mobile devices. Consider adding amax-width
in pixels or using responsive breakpoints to adjust the width for smaller screens.Example adjustment:
.public-text { width: 80vw; + max-width: 800px; font-size: 1.5rem; hyphens: none; }
app/templates/public/identity.hbs (1)
17-31
: Simplify boolean attribute syntax.The
@horizontal = true
syntax can be simplified to@horizontal={{true}}
for consistency with Ember's component argument conventions, though the current syntax is valid.Example (not requiring a diff, just for consistency):
@horizontal={{true}}app/components/header-nav.hbs (1)
40-48
: Consider using consistent dropdown item pattern.The new navigation links use
LinkTo
directly withclass="dropdown-item btn btn-primary"
, while the static pages above useddm.linkTo
from the dropdown menu API. This inconsistency may cause styling differences or unexpected behavior.Consider using the dropdown menu API for consistency:
<ddm.item> <ddm.linkTo @route='public.board' class="btn btn-primary"> {{t 'mixin.menuItems.board'}} </ddm.linkTo> </ddm.item>Or if intentional, verify that the styling remains consistent across all dropdown items.
translations/en.yaml (1)
88-144
: LGTM! Consider proofreading for consistency.The extensive new translations for board and identity sections are well-structured and provide comprehensive content for the new public pages. Consider a final proofreading pass to ensure tone and style consistency across all sections.
translations/nl.yaml (2)
162-169
: Complete the partners section translations.The partners section has multiple
LANG-TODO
placeholders that need to be populated before this can be considered production-ready. Since this is a WIP PR, this is expected, but please ensure these are completed before merging.Do you want me to help draft placeholder content for the partners section, or open a tracking issue for completing these translations?
98-121
: Consider breaking long story text into multiple lines.The board member stories (chair-story, secretary-story, treasurer-story, assessor1-story) are formatted as very long single lines. While YAML accepts this, breaking them into multiple lines using YAML's literal (
|
) or folded (>
) block scalar style would improve maintainability and readability in the source file.Example refactor for chair-story:
- chair-story: Hoi, ik ben Stefan en mag mezelf dit jaar Praeses noemen. Ik ben derdejaars student in Enschede en heb een half jaar IDE gestudeerd en heb er na een studieswitch nu een jaar Creative Business opzitten. Binnen Alpha ben ik de eindverantwoordelijke voor wat er gebeurt binnen de vereniging. Daarnaast zit ik vergadering voor, let ik erop dat het beleid goed wordt uitgevoerd, geef ik speeches en begeleid ik commissies en de andere bestuurders met hun taken. + chair-story: > + Hoi, ik ben Stefan en mag mezelf dit jaar Praeses noemen. Ik ben derdejaars student in Enschede en heb een half jaar IDE gestudeerd + en heb er na een studieswitch nu een jaar Creative Business opzitten. Binnen Alpha ben ik de eindverantwoordelijke voor wat er gebeurt binnen + de vereniging. Daarnaast zit ik vergadering voor, let ik erop dat het beleid goed wordt uitgevoerd, geef ik speeches en begeleid ik commissies + en de andere bestuurders met hun taken.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (31)
public/favicon128x128_blue.png
is excluded by!**/*.png
public/favicon128x128_red.png
is excluded by!**/*.png
public/favicon192x192_blue.png
is excluded by!**/*.png
public/favicon192x192_red.png
is excluded by!**/*.png
public/favicon48x48_blue.png
is excluded by!**/*.png
public/favicon48x48_red.png
is excluded by!**/*.png
public/favicon512x512_blue.png
is excluded by!**/*.png
public/favicon512x512_red.png
is excluded by!**/*.png
public/favicon64x64_blue.png
is excluded by!**/*.png
public/favicon64x64_red.png
is excluded by!**/*.png
public/favicon96x96_blue.png
is excluded by!**/*.png
public/favicon96x96_red.png
is excluded by!**/*.png
public/images/pakkerij.jpeg
is excluded by!**/*.jpeg
public/images/public/Identity.png
is excluded by!**/*.png
public/images/public/board/Alina.jpeg
is excluded by!**/*.jpeg
public/images/public/board/Alina.jpg
is excluded by!**/*.jpg
public/images/public/board/Jesse.jpeg
is excluded by!**/*.jpeg
public/images/public/board/Jesse.jpg
is excluded by!**/*.jpg
public/images/public/board/Niels.jpeg
is excluded by!**/*.jpeg
public/images/public/board/Niels.jpg
is excluded by!**/*.jpg
public/images/public/board/Stefan.jpeg
is excluded by!**/*.jpeg
public/images/public/board/Stefan.jpg
is excluded by!**/*.jpg
public/images/public/board/XL.jpeg
is excluded by!**/*.jpeg
public/images/public/board/XLI.jpeg
is excluded by!**/*.jpeg
public/images/public/board/XLII.jpeg
is excluded by!**/*.jpeg
public/images/public/board/XLIII.jpeg
is excluded by!**/*.jpeg
public/images/public/board/XLIV.jpeg
is excluded by!**/*.jpeg
public/images/public/board/XXXIX.jpeg
is excluded by!**/*.jpeg
public/images/public/board/banner.jpeg
is excluded by!**/*.jpeg
public/images/public/index/verantwoordelijkheid.jpg
is excluded by!**/*.jpg
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (25)
app/components/header-nav.hbs
(2 hunks)app/components/public/card.hbs
(1 hunks)app/components/public/index/about-alpha.hbs
(1 hunks)app/router.js
(1 hunks)app/routes/public/board.js
(1 hunks)app/routes/public/flux.js
(1 hunks)app/routes/public/identity.js
(1 hunks)app/routes/public/partners.js
(1 hunks)app/styles/app.scss
(2 hunks)app/styles/components/public/card.scss
(1 hunks)app/styles/components/public/header.scss
(2 hunks)app/styles/components/public/index/about-alpha.scss
(1 hunks)app/styles/routes/public.scss
(2 hunks)app/styles/routes/public/board.scss
(1 hunks)app/templates/public/board.hbs
(1 hunks)app/templates/public/flux.hbs
(1 hunks)app/templates/public/identity.hbs
(1 hunks)app/templates/public/partners.hbs
(1 hunks)tests/integration/components/public/card-test.js
(1 hunks)tests/unit/routes/public/board-test.js
(1 hunks)tests/unit/routes/public/flux-test.js
(1 hunks)tests/unit/routes/public/identity-test.js
(1 hunks)tests/unit/routes/public/partners-test.js
(1 hunks)translations/en.yaml
(6 hunks)translations/nl.yaml
(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
tests/unit/routes/public/board-test.js (5)
tests/unit/routes/books/new-test.js (2)
assert
(7-10)hooks
(4-11)tests/unit/routes/vacancies/new-test.js (2)
hooks
(4-11)assert
(7-10)tests/unit/routes/polls/new-test.js (1)
hooks
(4-11)tests/unit/routes/books/index-test.js (1)
hooks
(4-11)tests/unit/routes/articles/index-test.js (1)
assert
(7-10)
app/components/header-nav.hbs (2)
app/components/header-nav.js (4)
HeaderNav
(6-77)action
(50-60)pages
(65-74)page
(68-72)app/components/menu-sidebar.js (1)
action
(18-30)
tests/integration/components/public/card-test.js (1)
tests/rendering/components/advertisement-tool-test.js (1)
hooks
(6-19)
tests/unit/routes/public/flux-test.js (2)
tests/unit/routes/vacancies/new-test.js (2)
assert
(7-10)hooks
(4-11)tests/unit/routes/polls/index-test.js (2)
assert
(7-10)hooks
(4-11)
tests/unit/routes/public/identity-test.js (2)
tests/unit/routes/books/new-test.js (2)
assert
(7-10)hooks
(4-11)tests/unit/routes/books/index-test.js (1)
hooks
(4-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Lint
🔇 Additional comments (13)
app/styles/components/public/header.scss (1)
28-28
: LGTM!The change from
bottom
tocenter
aligns the header background image centrally, which complements the new public page designs introduced in this PR.app/styles/components/public/index/about-alpha.scss (1)
7-9
: LGTM!The simplification of
.about-v
styling aligns well with the component-based refactor, as the child element styling is now handled by thePublic::Card
component.app/styles/app.scss (1)
29-29
: LGTM!The new imports for the
public/card
component andpublic/board
route styles are correctly placed and follow the existing file organization conventions.Also applies to: 76-76
app/styles/routes/public/board.scss (1)
1-9
: LGTM!The styling provides appropriate spacing for board member cards and follows the nesting conventions used elsewhere in the codebase.
tests/unit/routes/public/flux-test.js (1)
1-11
: LGTM!The test follows established patterns from the codebase and correctly verifies the route exists using the modern Ember testing APIs.
app/routes/public/flux.js (1)
1-3
: LGTM!The route implementation is correct. This simple route class is appropriate when no custom hooks or data loading is required.
app/routes/public/identity.js (1)
1-3
: LGTM!The route implementation follows Ember conventions correctly.
app/styles/components/public/card.scss (1)
1-79
: Well-structured responsive component styling.The SCSS is well-organized with clear vertical/horizontal variants and appropriate responsive behavior for mobile screens. The use of flexbox and modern CSS features is appropriate.
tests/unit/routes/public/identity-test.js (1)
1-11
: LGTM!The test correctly verifies the route exists and follows the established testing patterns in the codebase.
app/routes/public/partners.js (1)
1-3
: LGTM!The route implementation is correct and follows Ember conventions.
app/router.js (1)
271-274
: LGTM!The new public routes are correctly defined and follow the existing router conventions. The routes align with the newly added templates and route modules.
app/styles/routes/public.scss (1)
33-33
: Ensure browser support for modern RGBA syntax The space-separated/slash formrgba(0 0 0 / 40%)
is supported in all evergreen browsers (Chrome, Edge, Firefox, Safari) but not in IE. If you must support IE, switch to the legacy comma-separatedrgba(0, 0, 0, 0.4)
.translations/nl.yaml (1)
259-259
: History header translation pending.The history header still contains
LANG-TODO
. Track this as pending work before considering the PR complete.
</ddm.linkTo> | ||
</ddm.item> | ||
{{/each-in}} | ||
{{! put links to other pages here }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Remove WIP comment before merging.
The comment "put links to other pages here" indicates this is still work in progress. Remove this comment before finalizing the PR.
- {{! put links to other pages here }}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
{{! put links to other pages here }} |
🤖 Prompt for AI Agents
In app/components/header-nav.hbs around line 39, remove the WIP Handlebars
comment "{{! put links to other pages here }}" before merging; either delete the
comment entirely or replace it with the actual navigation link markup (or a
brief TODO with an issue reference) so no development placeholder remains in the
final commit.
@@ -0,0 +1,8 @@ | |||
<div class="circle-card {{if @horizontal "horizontal" "vertical"}}"> | |||
<img class="card-image" src={{@imgurl}} alt=""> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add meaningful alt text for accessibility.
The empty alt=""
attribute makes the image decorative and invisible to screen readers. Since these images appear to be content-relevant (based on usage in board/identity pages), they should have descriptive alt text to ensure users with visual impairments can understand the card's purpose.
Consider passing alt text as a prop:
- <img class="card-image" src={{@imgurl}} alt="">
+ <img class="card-image" src={{@imgurl}} alt={{@altText}}>
Then update all usages to provide meaningful alt text describing the image content.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<img class="card-image" src={{@imgurl}} alt=""> | |
<img class="card-image" src={{@imgurl}} alt={{@altText}}> |
🤖 Prompt for AI Agents
In app/components/public/card.hbs around line 2, the image tag uses an empty alt
attribute which makes the image invisible to screen readers; change the template
to accept and use a passed-in alt prop (e.g., @alt or @altText) instead of
hardcoding alt="", update the <img> to set alt={{@alt}} (or alt={{@altText}})
and then update every invocation of this component to supply a meaningful
descriptive string for the image content so that images on board/identity pages
are accessible.
<Public::Header | ||
@imageUrl="/images/pakkerij.jpeg" | ||
@title="{{t 'component.public.flux.header'}}" | ||
@introTitle="{{t 'component.public.roomForum.intro-title'}}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify translation key usage is intentional.
The template uses component.public.roomForum.intro-title
for the Flux page's intro title. This mixes translation namespaces between flux
and roomForum
, which may be intentional if they share content, but could also indicate a copy-paste error. Verify this is correct or use component.public.flux.intro-title
for consistency.
Run this script to check if the translation key exists and is used elsewhere:
🏁 Script executed:
#!/bin/bash
# Check usage of roomForum.intro-title in Flux context
rg -n "component\.public\.roomForum\.intro-title" --type=hbs
Length of output: 199
Verify translation key definitions in locale files
Run this script to check for both keys outside templates:
🏁 Script executed:
#!/bin/bash
rg -n 'component\.public\.(flux|roomForum)\.intro-title' -g '!*.hbs'
Length of output: 0
Verify locale definitions for intro-title keys
Run this script to locate both keys in locale files:
🏁 Script executed:
#!/bin/bash
rg -n "component\.public\.(roomForum|flux)\.intro-title" -g "*.{yml,yaml,json}"
Length of output: 0
Verify locale definitions for intro-title
Run this script to find any definitions in YAML, JSON, JS, or TS files:
🏁 Script executed:
#!/bin/bash
rg -n "intro-title" -g "*.{yml,yaml,json,js,ts}"
Length of output: 497
Verify translation definitions for roomForum
and flux
Run this script:
🏁 Script executed:
#!/bin/bash
# Check roomForum translations
rg -n -C2 'roomForum' -g 'translations/*.{yaml,yml}'
# Check flux translations
rg -n -C2 'flux' -g 'translations/*.{yaml,yml}'
Length of output: 2678
Use Flux-specific translation key for introTitle
The Flux template is loading the Room Forum intro title (component.public.roomForum.intro-title
), resulting in mismatched copy. Reference the Flux translation—e.g. component.public.flux.intro
—or add a component.public.flux.intro-title
entry in your locale files.
🤖 Prompt for AI Agents
In app/templates/public/flux.hbs around line 5, the template uses the Room Forum
translation key component.public.roomForum.intro-title which causes incorrect
copy for the Flux page; replace that key with the Flux-specific key (for example
component.public.flux.intro or component.public.flux.intro-title) or add the new
component.public.flux.intro-title entry to the locale files and update
references accordingly so the template pulls the correct Flux translation.
intro: LANG-TODO | ||
our-partners: | ||
title: Our partners | ||
partner-1: LANG-TODO | ||
partner-2: LANG-TODO | ||
partner-3: LANG-TODO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Complete translations before merging.
The partners section contains multiple LANG-TODO
placeholders indicating incomplete translation work. Since this PR is marked as WIP, ensure these are completed with actual content before final merge.
Verify the partners section content is ready:
🏁 Script executed:
#!/bin/bash
# Find all LANG-TODO placeholders in translations
rg -n "LANG-TODO" translations/en.yaml translations/nl.yaml
Length of output: 2346
Replace all LANG-TODO
placeholders in translations/en.yaml before merging
Placeholders remain in partners, header, intro, events, and form sections—populate every entry with final text.
🤖 Prompt for AI Agents
In translations/en.yaml around lines 147-152 (and anywhere else in the file)
there are placeholder values set to LANG-TODO (notably intro, our-partners
partner-1/2/3 and other header/events/form entries); replace every LANG-TODO
with the final English copy for each key, ensuring the correct text for
partners, header, intro, events and form sections; keep YAML indentation/quoting
valid, preserve existing keys, and run the translation/i18n validation or
linting after changes to confirm no syntax or missing-key errors.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/templates/public/identity.hbs (1)
17-17
: Inconsistent attribute spacing.The
@horizontal = true
attributes have spaces around the equals sign, while@imgurl
and@text
do not. For consistency, remove the spaces.Apply this diff:
- @horizontal = true + @horizontal=trueApply the same change to lines 24 and 31.
Also applies to: 24-24, 31-31
translations/nl.yaml (2)
142-143
: Use proper YAML block scalar syntax for multi-line strings.The multi-line string starting at line 142 uses quoted syntax that spans lines, which can be fragile. Use YAML's folded block scalar (
>
) or literal block scalar (|
) for better readability and reliability.Apply this diff:
- intro: "Deze grondslag uit zich in de drie kernaspecten van de vereniging: vriendschap, vorming en verantwoordelijkheid. - Rondom deze drie kernaspecten is de vereniging opgebouwd en alles wat we doen sluit hierop aan:" + intro: > + Deze grondslag uit zich in de drie kernaspecten van de vereniging: vriendschap, vorming en verantwoordelijkheid. + Rondom deze drie kernaspecten is de vereniging opgebouwd en alles wat we doen sluit hierop aan:The
>
folded block scalar will automatically join lines with spaces and is the recommended YAML syntax for multi-line text blocks.
164-164
: Replace LANG-TODO placeholders with actual Dutch content.The partners section contains multiple
LANG-TODO
placeholders. Since this is a WIP PR, these should be completed before merging.Please ensure the following translation keys are completed:
template.public.partners.intro
(line 164)template.public.partners.our-partners.partner-1
(line 167)template.public.partners.our-partners.partner-2
(line 168)template.public.partners.our-partners.partner-3
(line 169)Do you need help drafting Dutch content for these sections, or would you like me to open an issue to track these missing translations?
Also applies to: 167-169
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/components/public/index/about-alpha.hbs
(1 hunks)app/templates/public/identity.hbs
(1 hunks)app/templates/public/partners.hbs
(1 hunks)translations/nl.yaml
(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/templates/public/partners.hbs
- app/components/public/index/about-alpha.hbs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lint
- GitHub Check: Test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
app/templates/public/identity.hbs (3)
14-18
: Remove spaces around the=
sign in the@horizontal
argument.Line 17 uses non-standard spacing (
@horizontal = true
) that is inconsistent with the rest of the component arguments and contradicts the fixes applied inabout-alpha.hbs
from previous reviews.Apply this diff:
<Public::Card @imgurl="/images/public/index/vriendschap.jpg" @text="{{t 'template.public.identity.three-vs.vriendschap'}}" - @horizontal = true + @horizontal=true />
21-25
: Remove spaces around the=
sign in the@horizontal
argument.Same spacing issue as the first card instance.
Apply this diff:
<Public::Card @imgurl="/images/public/index/vorming.jpg" @text="{{t 'template.public.identity.three-vs.vorming'}}" - @horizontal = true + @horizontal=true />
28-32
: Remove spaces around the=
sign in the@horizontal
argument.Same spacing issue as the previous card instances.
Apply this diff:
<Public::Card @imgurl="/images/public/index/verantwoordelijkheid.jpg" @text="{{t 'template.public.identity.three-vs.verantwoordelijkheid'}}" - @horizontal = true + @horizontal=true />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/components/public/index/about-alpha.hbs
(1 hunks)app/templates/public/identity.hbs
(1 hunks)app/templates/public/partners.hbs
(1 hunks)translations/nl.yaml
(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/templates/public/partners.hbs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lint
- GitHub Check: Test
🔇 Additional comments (1)
app/components/public/index/about-alpha.hbs (1)
9-27
: LGTM! Component refactoring is well-executed.The migration to the
Public::Card
component is clean and consistent. All three card instances correctly use the standard@prop=value
syntax without spaces around the equals sign, addressing the issues from previous reviews.
partners: | ||
header: Partners | ||
intro: LANG-TODO | ||
our-partners: | ||
title: Onze partners | ||
partner-1: LANG-TODO | ||
partner-2: LANG-TODO | ||
partner-3: LANG-TODO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Complete the partners section translations before release.
The partners section contains LANG-TODO
placeholders that will be visible to users if deployed. These should be replaced with actual Dutch translations.
Incomplete translations:
- Line 164:
intro: LANG-TODO
- Line 167:
partner-1: LANG-TODO
- Line 168:
partner-2: LANG-TODO
- Line 169:
partner-3: LANG-TODO
Would you like me to help draft placeholder text based on the English translations, or should this be deferred until actual partner content is available?
🤖 Prompt for AI Agents
In translations/nl.yaml around lines 162 to 169 the partners section still
contains LANG-TODO placeholders (intro at line 164, partner-1 at 167, partner-2
at 168, partner-3 at 169); replace each LANG-TODO with the correct Dutch
translations (or temporary Dutchized copies of the English source if the final
copy isn’t available), preserve YAML indentation and key names, keep phrasing
concise and user-facing, and commit the updated file with a clear message like
"i18n(nl): complete partners translations".
Branch for the new design for the external website.
Summary by CodeRabbit
New Features
Refactor
Style
Tests